Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Take into account rotation metadata to define video size #577

Merged
merged 5 commits into from
May 19, 2021
Merged

Take into account rotation metadata to define video size #577

merged 5 commits into from
May 19, 2021

Conversation

bobatsar
Copy link
Contributor

@bobatsar bobatsar commented Jun 1, 2017

  • ffmpeg autorotates videos but the video_size is unchanged,
    this results e.g. in stretched output images or even unreadable images or videos.
    This commit exchanges width and height if 90 or 270 degree rotation was detected.

@ziererf
On autorotate flip the video_size
1702841 
- ffmpeg autorotates videos but the video_size is unhanged,
  this results e.g. in stretched output images or even unreadable images or videos.
  This commit exchanges width and height if 90 or 270 degree rotation was detected.
@bobatsar
Copy link
Contributor Author

bobatsar commented Jun 1, 2017

See #212 and #529

@coveralls
Copy link

coveralls commented Jun 1, 2017 •
edited
Loading

Coverage Status

Coverage increased (+0.02%) to 68.625% when pulling ac6876a on bobatsar:auto_rotate into c8887f0 on Zulko:master.

@tburrows13 tburrows13 added the bug-fix For PRs and issues solving bugs. label Jun 6, 2017
kencochrane
kencochrane reviewed Jul 8, 2017
View reviewed changes
Copy link
Contributor

@kencochrane kencochrane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, Is there anyway to add a unit test, or show that it works?

@bobatsar
Copy link
Contributor Author

I'm planning to do a short test. Sorry for not replying earlier. But it can take some time. It's on my list.

@cvn cvn mentioned this pull request Dec 10, 2017
Closed
cvn added a commit to cvn/moviepy that referenced this pull request Dec 10, 2017
@cvn
Fix stretched vertical video, add ffmpeg_params to VideoFileClip
a562c01 
- FFMPEG_VideoReader: set size based on video rotation metadata
- use `-noautorotate` to disable autorotation (ffmpeg >=2.7 autorotates by default)
- closes Zulko#212, Zulko#577, Zulko#682
cvn added a commit to cvn/moviepy that referenced this pull request Dec 10, 2017
@cvn
Fix stretched vertical video, add ffmpeg_params to VideoFileClip
d1845d0 
- FFMPEG_VideoReader: set size based on video rotation metadata
- use `-noautorotate` to disable autorotation (ffmpeg >=2.7 autorotates by default)
- closes Zulko#212, closes Zulko#577, closes Zulko#682
@cvn cvn mentioned this pull request Dec 10, 2017
Closed
cvn added a commit to cvn/moviepy that referenced this pull request Dec 12, 2017
@cvn
Fix stretched vertical video, add ffmpeg_params to VideoFileClip
36bd3b0 
- add test to determine whether ffmpeg supports autorotate
- FFMPEG_VideoReader: set size based on video rotation metadata
- use `-noautorotate` to disable autorotation (ffmpeg >=2.7 autorotates by default)
- closes Zulko#212, closes Zulko#577, closes Zulko#682
cvn added a commit to cvn/moviepy that referenced this pull request Dec 12, 2017
@cvn
Fix stretched vertical video, add ffmpeg_params to VideoFileClip
48e47ad 
- add test to determine whether ffmpeg supports autorotate
- FFMPEG_VideoReader: set size based on video rotation metadata
- use `-noautorotate` to disable autorotation (ffmpeg >=2.7 autorotates by default)
- closes Zulko#212, closes Zulko#577, closes Zulko#682
@tburrows13 tburrows13 modified the milestones: Post 2.0.0, Release v2.0.0 Oct 4, 2020
@tburrows13
Copy link
Collaborator

Linking #683

@mondeja mondeja closed this Jan 25, 2021
@mondeja mondeja reopened this Jan 25, 2021
@mondeja
Copy link
Collaborator

mondeja commented Jan 25, 2021 •
edited
Loading

It seems to me that this is the right fix, but needs rewriting for the new ffmpeg output parser and a proper test case, maybe using this video or better creating multiples ones with ffmpeg at runtime.

@mondeja mondeja added lib-FFmpeg Issues pertaining to dependency FFmpeg. video Related to VideoClip and related classes, or handling of video in general. labels May 19, 2021
mondeja
mondeja approved these changes May 19, 2021
View reviewed changes
Copy link
Collaborator

@mondeja mondeja left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just merged it with master, added two tests and changed the place where the fix is being applied. I prefer just change the size in the FFMPEG_VideoReader instance so parsing ffmpeg -i output the metadata fits as much ffmpeg output as possible.

@mondeja mondeja changed the title On autorotate flip the video_size Take into account rotation metadata to define video size May 19, 2021
@mondeja
Copy link
Collaborator

mondeja commented May 19, 2021

4 years later but this is merged now 👍 In the future this could be improved defining optionally the rotation metadata writing files accordingly, (some preserve_rotation-like argument), but this fix is needed before.

@mondeja mondeja merged commit a472c86 into Zulko:master May 19, 2021
@mondeja mondeja mentioned this pull request Aug 18, 2021
Closed
@philihp
Copy link

philihp commented Feb 9, 2022

This was merged last year, but it has been almost 2 years since a new version was released. Could you release this?

@melyux
Copy link

melyux commented Feb 17, 2023

Please release this

@songge8
Copy link

songge8 commented Dec 10, 2023

Could you release this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kencochrane kencochrane kencochrane left review comments

@mondeja mondeja mondeja approved these changes

Assignees
No one assigned
Labels
bug-fix For PRs and issues solving bugs. lib-FFmpeg Issues pertaining to dependency FFmpeg. video Related to VideoClip and related classes, or handling of video in general.
Projects
None yet
Milestone
Release v2.0.0
Development

Successfully merging this pull request may close these issues.
9 participants
@bobatsar @coveralls @tburrows13 @mondeja @philihp @melyux @songge8 @kencochrane @ziererf